Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(react-components): migrate to new dx stage-1 #19040

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Jul 21, 2021

Pull request checklist

Pre-requirements

  • all vNext packages migrated

Description of changes

  • migrated react-components via nx migrate-converged-pkg migration generator
  • renamed FluentUIComponents/ to Concepts/ folder to mirror the stories IDs
  • fixed our custom storybook manager UI overrides that were broken after migration to sb 6.3
  • this also speed-up CI as api-extractor exec is faster ( haven't measured it though ) / 803e774

Followup - PR will cover

TODO:
  • static assets imported via relative paths are broken / 94cf859
    • 2021-07-21 at 13 23
    • 2021-07-21 at 13 25
  • ordering of stories is different than previously - need to be fixed / a22f090
    • 2021-07-21 at 13 26

Focus areas to test

@@ -177,6 +178,7 @@
"pretty-bytes": "5.6.0",
"raw-loader": "4.0.2",
"react-app-polyfill": "2.0.0",
"react-test-renderer": "16.8.6",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

propagating dev dep to single version policy

"bundle-size": "bundle-size measure",
"chromatic": "npx [email protected] --project-token $CHROMATIC_PROJECT_TOKEN --exit-zero-on-changes --build-script-name bundle:storybook",
"chromatic": "npx [email protected] --project-token $CHROMATIC_PROJECT_TOKEN --exit-zero-on-changes --build-script-name build-storybook",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this we will ship a proper production builds (build-storybook sets ENV to production

@@ -2,7 +2,7 @@
import * as React from 'react';
import { Source } from '@storybook/addon-docs/blocks';
import { createRenderer } from 'react-test-renderer/shallow';
import { CodeExample } from './utils';
import { CodeExample } from './utils.stories';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed migrated storybook related files to include stories suffix

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 21, 2021

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-accordion
Accordion (including children components)
75.787 kB
22.366 kB
react-avatar
Avatar
56.558 kB
15.66 kB
react-badge
Badge
24.343 kB
7.165 kB
react-badge
CounterBadge
27.156 kB
7.851 kB
react-badge
PresenseBadge
237 B
177 B
react-button
Button
25.016 kB
8.035 kB
react-button
CompoundButton
30.308 kB
8.911 kB
react-button
MenuButton
26.603 kB
8.543 kB
react-button
ToggleButton
34.613 kB
8.671 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
177.666 kB
50.281 kB
react-components
react-components: FluentProvider & webLightTheme
36.288 kB
11.615 kB
react-divider
Divider
15.889 kB
5.747 kB
react-image
Image
10.642 kB
4.264 kB
react-label
Label
9.397 kB
3.839 kB
react-link
Link
14.715 kB
6.012 kB
react-make-styles
makeStaticStyles (runtime)
7.59 kB
3.321 kB
react-make-styles
makeStyles + mergeClasses (runtime)
22.135 kB
8.356 kB
react-make-styles
makeStyles + mergeClasses (build time)
2.557 kB
1.202 kB
react-menu
Menu (including children components)
115.15 kB
34.665 kB
react-menu
Menu (including selectable components)
117.88 kB
35.217 kB
react-popover
Popover
124.599 kB
36.198 kB
react-portal
Portal
7.78 kB
2.672 kB
react-positioning
usePopper
23.145 kB
7.942 kB
react-provider
FluentProvider
16.286 kB
5.991 kB
react-text
Text - Default
11.798 kB
4.452 kB
react-text
Text - Wrappers
15.414 kB
4.734 kB
react-theme
Teams: all themes
32.941 kB
6.674 kB
react-theme
Teams: Light theme
20.247 kB
5.662 kB
react-tooltip
Tooltip
46.054 kB
15.658 kB
react-utilities
SSRProvider
213 B
170 B
🤖 This report was generated against 28c579ac8d2f6128d33ed1a3357ca6ba6f2ee7f5

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 21, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1eb466e:

Sandbox Source
Fluent UI React Starter Configuration

@size-auditor
Copy link

size-auditor bot commented Jul 21, 2021

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 28c579ac8d2f6128d33ed1a3357ca6ba6f2ee7f5 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 21, 2021

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
Panel mount 2062 1383 1000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 929 907 5000
BaseButton mount 893 880 5000
Breadcrumb mount 2671 2674 1000
ButtonNext mount 438 438 5000
Checkbox mount 1494 1507 5000
CheckboxBase mount 1355 1272 5000
ChoiceGroup mount 4767 4685 5000
ComboBox mount 963 986 1000
CommandBar mount 10256 10361 1000
ContextualMenu mount 6209 6339 1000
DefaultButton mount 1101 1144 5000
DetailsRow mount 3662 3686 5000
DetailsRowFast mount 3689 3652 5000
DetailsRowNoStyles mount 3517 3543 5000
Dialog mount 2144 2148 1000
DocumentCardTitle mount 162 135 1000
Dropdown mount 3275 3213 5000
FluentProviderNext mount 7586 7512 5000
FocusTrapZone mount 1769 1795 5000
FocusZone mount 1812 1804 5000
IconButton mount 1741 1775 5000
Label mount 337 345 5000
Layer mount 1783 1749 5000
Link mount 458 455 5000
MakeStyles mount 1813 1834 50000
MenuButton mount 1466 1465 5000
MessageBar mount 2045 2060 5000
Nav mount 3250 3226 1000
OverflowSet mount 1117 1108 5000
Panel mount 2062 1383 1000 Possible regression
Persona mount 836 820 1000
Pivot mount 1416 1425 1000
PrimaryButton mount 1282 1327 5000
Rating mount 7687 7542 5000
SearchBox mount 1326 1309 5000
Shimmer mount 2523 2510 5000
Slider mount 1954 1985 5000
SpinButton mount 4933 5049 5000
Spinner mount 420 431 5000
SplitButton mount 3180 3162 5000
Stack mount 505 508 5000
StackWithIntrinsicChildren mount 1556 1507 5000
StackWithTextChildren mount 4528 4515 5000
SwatchColorPicker mount 10174 10206 5000
Tabs mount 1389 1399 1000
TagPicker mount 2561 2618 5000
TeachingBubble mount 11895 11985 5000
Text mount 423 409 5000
TextField mount 1368 1335 5000
ThemeProvider mount 1194 1216 5000
ThemeProvider virtual-rerender 623 585 5000
Toggle mount 808 815 5000
buttonNative mount 120 119 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AlertMinimalPerf.default 287 254 1.13:1
DividerMinimalPerf.default 368 342 1.08:1
ListWith60ListItems.default 669 620 1.08:1
RefMinimalPerf.default 241 225 1.07:1
BoxMinimalPerf.default 354 336 1.05:1
ChatDuplicateMessagesPerf.default 306 291 1.05:1
HeaderMinimalPerf.default 363 346 1.05:1
SkeletonMinimalPerf.default 367 350 1.05:1
AccordionMinimalPerf.default 149 143 1.04:1
ImageMinimalPerf.default 379 366 1.04:1
ItemLayoutMinimalPerf.default 1215 1166 1.04:1
LabelMinimalPerf.default 390 375 1.04:1
RosterPerf.default 1177 1128 1.04:1
AttachmentMinimalPerf.default 153 149 1.03:1
GridMinimalPerf.default 327 319 1.03:1
PopupMinimalPerf.default 602 583 1.03:1
TableMinimalPerf.default 405 392 1.03:1
AnimationMinimalPerf.default 404 397 1.02:1
AvatarMinimalPerf.default 187 183 1.02:1
ButtonOverridesMissPerf.default 1709 1672 1.02:1
ChatWithPopoverPerf.default 363 356 1.02:1
DialogMinimalPerf.default 739 727 1.02:1
DropdownMinimalPerf.default 3135 3080 1.02:1
FlexMinimalPerf.default 290 283 1.02:1
ReactionMinimalPerf.default 368 361 1.02:1
TreeMinimalPerf.default 791 776 1.02:1
VideoMinimalPerf.default 621 610 1.02:1
AttachmentSlotsPerf.default 1084 1073 1.01:1
CardMinimalPerf.default 545 539 1.01:1
ChatMinimalPerf.default 640 634 1.01:1
CheckboxMinimalPerf.default 2744 2715 1.01:1
DatepickerMinimalPerf.default 5509 5466 1.01:1
EmbedMinimalPerf.default 4112 4063 1.01:1
FormMinimalPerf.default 400 395 1.01:1
InputMinimalPerf.default 1268 1260 1.01:1
LayoutMinimalPerf.default 356 353 1.01:1
ListCommonPerf.default 617 608 1.01:1
RadioGroupMinimalPerf.default 439 436 1.01:1
SegmentMinimalPerf.default 346 344 1.01:1
SplitButtonMinimalPerf.default 3777 3748 1.01:1
StatusMinimalPerf.default 684 676 1.01:1
TableManyItemsPerf.default 1891 1865 1.01:1
ToolbarMinimalPerf.default 920 915 1.01:1
ButtonSlotsPerf.default 545 546 1:1
CarouselMinimalPerf.default 460 458 1:1
DropdownManyItemsPerf.default 679 678 1:1
ListMinimalPerf.default 500 502 1:1
LoaderMinimalPerf.default 682 683 1:1
PortalMinimalPerf.default 178 178 1:1
ProviderMergeThemesPerf.default 1677 1673 1:1
ProviderMinimalPerf.default 988 984 1:1
SliderMinimalPerf.default 1549 1545 1:1
TextMinimalPerf.default 343 344 1:1
CustomToolbarPrototype.default 3834 3843 1:1
MenuMinimalPerf.default 831 836 0.99:1
TextAreaMinimalPerf.default 480 484 0.99:1
HeaderSlotsPerf.default 762 778 0.98:1
IconMinimalPerf.default 600 615 0.98:1
TooltipMinimalPerf.default 990 1011 0.98:1
TreeWith60ListItems.default 169 173 0.98:1
ListNestedPerf.default 528 545 0.97:1
MenuButtonMinimalPerf.default 1598 1655 0.97:1
ButtonMinimalPerf.default 162 169 0.96:1

@PeterDraex PeterDraex removed the Status: Blocked Resolution blocked by another issue label Jul 23, 2021
@Hotell Hotell force-pushed the hotell/build-system/migrate-react-components-to-new-dx-1 branch from 3267051 to edfb0fa Compare August 9, 2021 09:19
@@ -279,6 +281,7 @@
"dependencies": [
"@babel/core",
"@babel/preset-typescript",
"@types/react-test-renderer",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syncpack opt out behaviour as it doesn't understand single version policy and devDeps - note the version are the same in all packages having this specified as devDep

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If syncpack can be modified to help with that I'm happy to take a look if you'd like to open an issue to explain this use case.

Copy link
Contributor Author

@Hotell Hotell Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @JamieMason , thanks for chiming in ! Once this is merged and we'll have some extra time , we gonna submit an issue. cheers

Btw, just out of curiosity how did you managed to notice in a draft PR that there was mention on syncpack ? :D

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, sounds good 👍🏻

Btw, just out of curiosity how did you managed to notice in a draft PR that there was mention on syncpack ? :D

from github code/issues search it's (admittedly kind of sad but also) a handy way to find out about problems or feature feedback 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha nice, thanks for sharing!

@@ -301,6 +304,7 @@
"gzip-size",
"jju",
"pretty-bytes",
"react-test-renderer",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as comment above

@@ -0,0 +1,37 @@
import { create } from '@storybook/theming';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from react-examples/.storybook

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of duplicating the custom styling files, let's just delete them from react-examples since having the customized storybook styling there isn't important.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll address those in PR followup
2021-08-13 at 10 42

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would have been nice to do that together with this change so it would show up as a rename not a new file (at least in git tools that are configured to handle renames)

},
});

function getVnextStories() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom logic to parse for component stories that are part of react-components suite

@@ -0,0 +1,111 @@
<!--
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • copied from react-examples/.storybook
  • updated docs + fixed selectors

@@ -0,0 +1,8 @@
import { addons } from '@storybook/addons';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from react-examples/.storybook

/**
* @see https://storybook.js.org/docs/react/writing-stories/naming-components-and-hierarchy#sorting-stories
*/
order: [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partially copied from react-examples/.storybook and used latest SB syntax for "ordering"

"start": "yarn storybook",
"docs": "api-extractor run --config=config/api-extractor.local.json --local",
"build:local": "tsc -p . --module esnext --emitDeclarationOnly && node ../../scripts/typescript/normalize-import --output dist/react-components/src && yarn docs",
"storybook": "start-storybook --port 3000 -s ./public",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we override sb default static assets behaviour -> static assets are consumed from ./public instead of ./static

@@ -0,0 +1,4 @@
<svg width="76" height="28" viewBox="0 0 76 28" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied from react-examples/.storybook

*/
declare module '*.scss' {
const styles: { [className: string]: string };
export default styles;
}
declare module '*.svg' {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

importing svg was type error in both react-exampels and react-components

@Hotell
Copy link
Contributor Author

Hotell commented Aug 11, 2021

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

"baseUrl": ".",
"target": "ES5",
"module": "CommonJS",
"lib": ["ES2016", "dom"],
Copy link
Contributor Author

@Hotell Hotell Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like ES2015 won't be enough as we are using ES2015+ features (Array.prototype.includes for instance etc )

We should update migration generator to this change as well - cc @ling1726

tracked #19367

@Hotell Hotell marked this pull request as ready for review August 11, 2021 12:43
@Hotell Hotell force-pushed the hotell/build-system/migrate-react-components-to-new-dx-1 branch from 603686a to de9f402 Compare August 13, 2021 09:47
@behowell behowell assigned miroslavstastny and unassigned Hotell Aug 19, 2021
@Hotell Hotell force-pushed the hotell/build-system/migrate-react-components-to-new-dx-1 branch from cebf5cc to cdfcc64 Compare August 19, 2021 16:34
@Hotell Hotell force-pushed the hotell/build-system/migrate-react-components-to-new-dx-1 branch from cdfcc64 to 5552787 Compare August 19, 2021 16:42
@Hotell Hotell force-pushed the hotell/build-system/migrate-react-components-to-new-dx-1 branch from 3c6ab2e to 1eb466e Compare August 19, 2021 18:38
@Hotell Hotell enabled auto-merge (squash) August 19, 2021 18:39
@Hotell Hotell merged commit ef0f778 into microsoft:master Aug 19, 2021
@PeterDraex
Copy link
Contributor

🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants